-
Notifications
You must be signed in to change notification settings - Fork 143
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Guarantee at least 10 peers to broadcast #767
Guarantee at least 10 peers to broadcast #767
Conversation
2840235
to
90826dd
Compare
90826dd
to
f890919
Compare
f890919
to
fd71502
Compare
fd71502
to
1f67ab8
Compare
Codecov Report
@@ Coverage Diff @@
## master #767 +/- ##
==========================================
+ Coverage 86.3% 86.48% +0.18%
==========================================
Files 225 225
Lines 19280 19363 +83
==========================================
+ Hits 16640 16747 +107
+ Misses 1438 1413 -25
- Partials 1202 1203 +1
|
1f67ab8
to
45013ed
Compare
- If there are less than 10 peers in the routing table, select all peers. | ||
- If there are more than 10 peers in the routing table, | ||
choose one from each bucket, and if the number is less than 10, | ||
then select an additional peers so that the total is 10. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As it's not a manual or docs, rather than explaining its behavioral details, we'd better to write a short descriptive guarantee and its intention.
@@ -248,10 +252,14 @@ public void BroadcastTestMessage(Address? except, string data) | |||
public void BroadcastMessage(Address? except, Message message) | |||
{ | |||
var peers = Protocol.PeersToBroadcast(except).ToList(); | |||
var peersString = peers.Aggregate( | |||
new StringBuilder(), | |||
(builder, peer) => builder.Append($"{peer.Address.ToHex()}, ")).ToString(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't the same thing be easily achieved using string.Join()
method?
If the number of total peers in routing table is less then 10, broadcast messages to all peers.
If not, pick one from each bucket, and pick additional peers which were not selected in first step until total peers count reach 10.
Closes #765.